Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Producers support staged tx #961

Merged
merged 18 commits into from
Jan 10, 2025
Merged

Producers support staged tx #961

merged 18 commits into from
Jan 10, 2025

Conversation

James-Mart
Copy link
Member

No description provided.

@James-Mart James-Mart added the System app Related to system services and their apps/plugins label Dec 13, 2024
@James-Mart James-Mart requested a review from swatanabe December 13, 2024 22:19
@James-Mart James-Mart marked this pull request as draft December 13, 2024 22:24
@James-Mart
Copy link
Member Author

This is in draft because before delivering I need to:

  • Add unit tests - I guess they will have to be in python?
  • Answer some questions:
    • Shouldn't AuthStack be used in checkAuthSys too?
    • Is one AuthStack instance per service sufficient? I think it might be okay to share the same stack between isAuth and isReject, but perhaps need a different stack if this is also added to checkAuthSys...

@swatanabe
Copy link
Collaborator

  • Shouldn't AuthStack be used in checkAuthSys too?

Recursion in account auth doesn't really work with checkAuthSys. Recursion only works with a multisig that allows failure of some nested calls without aborting.

  • Is one AuthStack instance per service sufficient? I think it might be okay to share the same stack between isAuth and isReject, but perhaps need a different stack if this is also added to checkAuthSys...

Technically what we want is one auth stack per top-level call. Actually, I think the easiest way to get fully correct semantics is to pass the stack as a parameter. An auth service that delegates would need to add the current account to the stack before the nested calls. Non-delegating auth services can safely leave off the extra parameter, and the extra data will be ignored. The extra size will not be a problem, because the wasm stack will overflow first (I think the current limit is 255 recursive actions).

@James-Mart James-Mart marked this pull request as ready for review January 3, 2025 20:22
.clang-format Outdated Show resolved Hide resolved
services/system/AuthDelegate/src/AuthDelegate.cpp Outdated Show resolved Hide resolved
services/system/AuthSig/test/src/AuthSig-test.cpp Outdated Show resolved Hide resolved
services/system/AuthSig/test/src/AuthSig-test.cpp Outdated Show resolved Hide resolved
services/system/AuthSig/test/src/AuthSig-test.cpp Outdated Show resolved Hide resolved
services/system/Producers/src/Producers.cpp Outdated Show resolved Hide resolved
services/system/Producers/src/Producers.cpp Outdated Show resolved Hide resolved
services/system/Producers/test/src/Producers-test.cpp Outdated Show resolved Hide resolved
@James-Mart James-Mart merged commit 7953978 into main Jan 10, 2025
3 checks passed
@James-Mart James-Mart deleted the producers-support-staged-tx branch January 10, 2025 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
System app Related to system services and their apps/plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants